Skip to content

warn on ambiguous -p plugin module usage#8

Open
yashwant86 wants to merge 6 commits intomainfrom
pr-14270
Open

warn on ambiguous -p plugin module usage#8
yashwant86 wants to merge 6 commits intomainfrom
pr-14270

Conversation

@yashwant86
Copy link
Copy Markdown

@yashwant86 yashwant86 commented Apr 7, 2026

Mirror of pytest-dev#14270


Summary by MergeMonkey

  • Documentation:
    • Added changelog entry describing the new ambiguous plugin module warning
  • Just Shipped:
    • Warns users when `-p` loads a module with no pytest hooks but a matching entry-point plugin exists, suggesting the correct plugin name

@mergemonkeyhq
Copy link
Copy Markdown

mergemonkeyhq bot commented Apr 11, 2026

Risk AssessmentNEEDS-TESTING · ~15 min review

Focus areas: plugin hook detection logic · entry-point scanning correctness · warning message accuracy

Assessment: Adds warning logic for ambiguous plugin module loading via -p flag.

Walkthrough

When a user runs pytest with --disable-plugin-autoload -p <module_name>, the system now checks whether the loaded module contains any pytest hook implementations. If it doesn't, it scans all installed pytest11 entry-points for ones whose module path starts with the given module name, and emits a warning suggesting the correct -p name to use instead.

Changes

Files Summary
Changelog
changelog/14135.bugfix.rst
New changelog entry documenting the ambiguous `-p` plugin module warning behavior.
Ambiguous plugin detection logic
src/_pytest/config/__init__.py
Introduces `_warn_about_submodule_entrypoint_plugin` to detect when `-p` loads a module lacking pytest hooks while a `pytest11` entry-point with a submodule path exists, and `_plugin_has_pytest_hooks` to introspect whether a plugin defines any hook implementations. Emits a `PytestConfigWarning` suggesting the correct `-p` name.
Tests for ambiguous plugin warning
testing/test_config.py
Adds five test cases covering: warning emitted for single and multiple submodule entry-points, no warning when the module has hooks, no warning when no matching submodule entry-point exists, and a direct unit test of the warning method.

Sequence Diagram

sequenceDiagram
    participant User
    participant Config as PytestPluginManager
    participant EntryPoints as importlib.metadata
    participant HookCheck as _plugin_has_pytest_hooks
    participant WarnMethod as _warn_about_submodule_entrypoint_plugin

    User->>Config: import_plugin(modname, consider_entry_points=True)
    Config->>Config: load_setuptools_entrypoints("pytest11", name=modname)
    alt entry-point loaded
        Config-->>User: return (plugin registered via entry-point)
    else no entry-point match
        Config->>Config: __import__(modname)
        Config->>Config: register(mod, modname)
        Config->>WarnMethod: _warn_about_submodule_entrypoint_plugin(modname, mod)
        WarnMethod->>HookCheck: _plugin_has_pytest_hooks(mod)
        alt module has hooks
            HookCheck-->>WarnMethod: True
            WarnMethod-->>Config: return (no warning)
        else module has no hooks
            HookCheck-->>WarnMethod: False
            WarnMethod->>EntryPoints: distributions()
            EntryPoints-->>WarnMethod: list of distributions with entry-points
            alt matching submodule entry-points found
                WarnMethod->>User: emit PytestConfigWarning with suggestion
            else no matching entry-points
                WarnMethod-->>Config: return (no warning)
            end
        end
    end
Loading

Dig Deeper With Commands

  • /review <file-path> <function-optional>
  • /chat <file-path> "<question>"
  • /roast <file-path>

Runs only when explicitly triggered.

if ep.group == "pytest11"
and ep.name != modname
and isinstance(getattr(ep, "value", None), str)
and getattr(ep, "value").split(":", 1)[0].startswith(modname_prefix)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Iterates all installed distributions on every ambiguous check

importlib.metadata.distributions() scans all installed packages. This runs every time a -p loaded module has no hooks. Consider caching the result or using importlib.metadata.entry_points(group='pytest11') which is more targeted and faster on Python 3.12+.

if ep.group == "pytest11"
and ep.name != modname
and isinstance(getattr(ep, "value", None), str)
and getattr(ep, "value").split(":", 1)[0].startswith(modname_prefix)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double getattr call risks AttributeError on race

Line 920 checks getattr(ep, 'value', None) but line 921 calls getattr(ep, 'value') without default. If value is a property that could change or raise, this could throw. Use a single assignment: val = getattr(ep, 'value', None) then check isinstance(val, str) and val.split(...)....

@mergemonkeyhq
Copy link
Copy Markdown

mergemonkeyhq bot commented Apr 11, 2026

Actionable Comments Posted: 2

🧹 Nitpick comments (1)
\`\_plugin\_has\_pytest\_hooks\` has no explicit return type path - src/_pytest/config/__init__.py (940)
The function relies on \`any\(\)\` which always returns bool, so it's correct. However, \`parse\_hookimpl\_opts\` may raise exceptions for certain attributes \(e.g., descriptors\). Consider wrapping with try/except to avoid unexpected failures during introspection.
🧾 Coverage Summary
✔️ Covered (3 files)
- changelog/14135.bugfix.rst
- src/_pytest/config/__init__.py
- testing/test_config.py

@yashwant86
Copy link
Copy Markdown
Author

/review --force

@mergemonkeyhq
Copy link
Copy Markdown

mergemonkeyhq bot commented Apr 11, 2026

Actionable Comments Posted: 0

👀 Worth a Look (1)
[MEDIUM] Hook detection can still raise on exotic plugin attributes - src/_pytest/config/__init__.py (938)
The new warning path calls \`\_plugin\_has\_pytest\_hooks\(\)\` on every \`-p\` module import that didn't resolve through entry points. That helper walks \`dir\(plugin\)\` and then \`parse\_hookimpl\_opts\(\)\` does an unguarded \`getattr\(plugin, name\)\` for every \`pytest\_\*\` attribute, so a plugin exposing a problematic descriptor/property can now fail during warning detection instead of just being imported.

Wrap the hook probe in a small \`try/except Exception\` around \`parse\_hookimpl\_opts\(\)\`/\`getattr\`, or limit detection to safe attribute sources for modules so the warning path can't break plugin import.
🧹 Nitpick comments (1)
Entry-point \`value\` is read twice in the new suggestion filter - src/_pytest/config/__init__.py (920)
The suggestion scan guards \`getattr\(ep, "value", None\)\` and then immediately reads \`getattr\(ep, "value"\)\` again. For normal \`importlib.metadata.EntryPoint\` objects that's fine, but the code now assumes repeated access is stable; a custom entry-point-like object with a property or side effect can pass the first check and fail or change on the second.

Store \`value = getattr\(ep, "value", None\)\` once inside the loop/comprehension and use that cached value for both the type check and split.
🧾 Coverage Summary
✔️ Covered (3 files)
- changelog/14135.bugfix.rst
- src/_pytest/config/__init__.py
- testing/test_config.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants